feat(code-blocks): add language picker #429
Conversation
🦋 Changeset detectedLatest commit: 83592f1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for stacks-editor ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
NOTE this only works in tests at the moment, need to figure out adding the plugin to the real editor
There was a problem hiding this comment.
Nice enhancement @alexwarren. This is something I've wanted personally, so I'm selfishly happy to see this finally coming.
I've performed a partial review and made some suggestions on the markup. Happy to discuss the suggestion further if you'd like a little Stacks/accessibility help.
I also noticed two bugs:
- Any string can be set as the language
- Languages not included in the predefined languages are set unexpectedly.
Note
After reviewing, I realized this was mentioned in the description:
Or keep typing if the language is not listed, and press Enter.
I'd consider this on that bug/feature line but I wouldn't cry if this stayed as-is.
These two bugs probably shouldn't be show-stoppers, but I figured they were worth mentioning.
| <div class="ps-absolute t24 r4 js-language-input"> | ||
| <div class="ps-relative mb8"> | ||
| <label class="v-visible-sr" for="example-search">Search</label> | ||
| <input type="text" class="s-input s-input__search fs-caption js-language-input-textbox" placeholder="Search for a language" contenteditable="false" /> | ||
| <span class="s-input-icon s-input-icon__search svg-icon-bg iconSearchSm"></span> | ||
| </div> | ||
| <div class="s-card fs-caption c-pointer py4 px4 js-language-dropdown-container"> | ||
| <ul class="s-menu js-language-dropdown"></ul> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Suggestion: Use .s-popover as the container for the language selector
Note
FWIW, I'm not 100% on this suggestion. I'd suggest running it by a designer.
It seems we've added some custom code to show/hide the language selector when we could use the functionality built into Stacks by instead using the .s-popover component. This would allow us to simplify the functional code in this file and get the benefits that come from using the popover (things like automatic layout). This would potentially change the design of the language selector unless we add a bunch of atomic styling classes to the popover, but there could be a styling middle-ground that allows us to use the .s-popover element
There was a problem hiding this comment.
ProseMirror doesn't play nicely with things that try to change HTML so it won't play nicely with the built-in popover component. It really needs the state (i.e. whether the popover is open or not) to be in ProseMirror, which is why we're manually controlling it here, rather than letting anything external handle that. Combined with the different design here, I don't think using .s-popover would work well for this.
dancormier
left a comment
dancormier
left a comment
There was a problem hiding this comment.
Thanks for making the UI changes @alexwarren. They work well. I only found a couple of minor issues (one mentioned inline; the other here) that I don't consider blockers 🎉
Return creates new line unexpectedly
When pressing return when inputting an arbitrary string into the language selector, a new line is added to the editor. In this gif, I've removed most of what comes before the code block to demonstrate:
| textbox.addEventListener( | ||
| "mousedown", | ||
| this.onLanguageInputMouseDown.bind(this) | ||
| ); |
There was a problem hiding this comment.
Is this listener necessary? What's the case where the event of a user clicking in the field needs to be prevented?
There was a problem hiding this comment.
It prevents ProseMirror from erroring when you triple-click the textbox (as you might if you want to select all the text in it). Added a comment so this is clearer.
Thanks, I've checked in a fix for this too. |
This PR adds a language picker to code blocks.
Create a code block, and there is a new drop-down icon to the right of the auto-detected language:
Click the language, or press
Ctrl+;(PC) orCmd+;(Mac) to open the language picker:Start typing to filter down the list of languages:
Click a language, or select one with the arrow keys to set it. Or keep typing if the language is not listed, and press Enter.
The language of the code block is updated, and the relevant syntax highlighting is applied:
In Markdown view, the language has been set:
For demo purposes there is a hard-coded list of languages. For production use on Stack Overflow, we'll inject the list of languages as part of the highlighting options when instantiating the editor.